Skip to content

feat(podman): honor OCI image working directories - #2563

Open
matthewgrossman wants to merge 5 commits into
mainfrom
feat/2526-oci-working-dir-podman/matthewgrossman
Open

feat(podman): honor OCI image working directories#2563
matthewgrossman wants to merge 5 commits into
mainfrom
feat/2526-oci-working-dir-podman/matthewgrossman

Conversation

@matthewgrossman

@matthewgrossman matthewgrossman commented Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Extend the Docker/shared OCI working-directory support introduced in #2530 to the Podman driver. Podman validates the original immutable image before its workspace volume masks the OCI workdir, then launches the sandbox at the validated path without expanding the image user filesystem authority.

This work was originally developed on top of #2530. That PR has merged, and this PR now targets main.

Related Issue

Part of #2526

Changes

  • resolve the immutable image ID, OCI identity, and workdir once for both probe and final launch
  • validate non-default OCI workdirs in a resource-limited, networkless probe without the workspace volume, secrets, or sandbox token
  • preserve cancellation-safe cleanup and reconcile labeled probes after gateway restarts
  • attest the validated image identity before the final supervisor prepares the managed workspace
  • mount Podman persistence at the resolved OCI workdir and reject conflicting driver-config mounts
  • bound and sanitize probe diagnostics
  • cover Podman OCI identity, supplementary groups, resource limits, cleanup, nested volumes, and forged validation metadata
  • document the shared no-authority-expansion invariant without duplicating implementation detail

Testing

  • mise run pre-commit
  • mise run test
  • strict Clippy for affected crates with -D warnings
  • focused Podman, supervisor-process, sandbox, and server tests
  • E2E coverage added for Podman OCI workdir and identity behavior

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture and user documentation updated

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

🌿 Preview your docs: https://nvidia-preview-pr-2563.docs.buildwithfern.com/openshell

@matthewgrossman matthewgrossman mentioned this pull request Jul 30, 2026
Merged
8 tasks
@matthewgrossman
matthewgrossman force-pushed the feat/2526-oci-working-dir-podman/matthewgrossman branch 2 times, most recently from a7e1ee7 to a6993fa Compare August 3, 2026 17:55
Base automatically changed from feat/2526-oci-working-dir/matthewgrossman to main August 4, 2026 17:51
@matthewgrossman
matthewgrossman force-pushed the feat/2526-oci-working-dir-podman/matthewgrossman branch from a6993fa to a746233 Compare August 4, 2026 18:06
@matthewgrossman
feat(podman): honor OCI image working directories
2ab6ea3 
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman
matthewgrossman force-pushed the feat/2526-oci-working-dir-podman/matthewgrossman branch from a746233 to 2ab6ea3 Compare August 4, 2026 18:07
@matthewgrossman
fix(podman): harden workspace probe lifecycle
d7d5cf7 
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman matthewgrossman added the test:e2e Requires end-to-end coverage label Aug 4, 2026
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Label test:e2e applied for d7d5cf7. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@matthewgrossman
refactor(podman): simplify workspace probe flow
e1f6745 
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman
refactor(podman): clarify request execution flow
5348906 
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
@matthewgrossman
fix(podman): recover ambiguous workspace probes
2069f04 
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
matthewgrossman
matthewgrossman commented Aug 5, 2026
View reviewed changes

@matthewgrossman matthewgrossman left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gator-agent

PR Review Status

Validation: This PR is project-valid because it implements maintainer-authored, agent-ready issue #2526 and is scoped to the Podman half of the OCI WorkingDir work after #2530 merged.
Head SHA: 2069f04785a9ae0742057684c25f479176e0902e
Base SHA: 490f66f47af4c225fabeb2dc48ea51c67154827c
Merge base SHA: 490f66f47af4c225fabeb2dc48ea51c67154827c
Patch ID: af1aa85fb495511ae397c200658e15602169ca7b
Gator payload: 2
Review mode: initial
Previous reviewed SHA: none

Blocking findings:

  • GATOR-2069f047-01: Podman workspace-probe startup recovery can remove containers selected only by the forgeable openshell.workspace-probe=true label.

Carried findings:

  • None

Non-blocking suggestions:

  • None

Docs: Fern docs and architecture guidance were updated for the user-visible compute-driver behavior.

Next state: gator:in-review

Comment thread crates/openshell-driver-podman/src/driver.rs
"Bridge network ready"
);

let removed_probes = remove_workspace_probes(&client, config.stop_timeout_secs).await?;

@matthewgrossman matthewgrossman Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gator-agent

Warning - GATOR-2069f047-01

Invariant: Podman workspace-probe recovery must only remove stale containers that OpenShell can identify as its own probes; it must not force-remove every container on the configured Podman socket that carries a public label.

Prerequisite: A local operator, another OpenShell gateway, or any user sharing the configured Podman engine can have a container with the forgeable or accidentally reused label openshell.workspace-probe=true.

Entry point -> sink: Starting or restarting a gateway configured with the Podman driver calls PodmanComputeDriver::new, which reaches remove_workspace_probes; that lists by LABEL_WORKSPACE_PROBE_FILTER and then calls remove_container with forced removal and volume cleanup.

Base -> head: The base tree did not delete Podman containers during startup based on a workspace-probe label. This head removes every container returned for openshell.workspace-probe=true; the probe spec omits the normal openshell.managed ownership label and cleanup does not check name, state, gateway instance, or sandbox ownership.

Impact: A gateway restart can delete an unrelated user container, including its anonymous volumes, or a second gateway can delete another gateway's active validation probe and make a valid sandbox creation fail.

Reproducer: Run podman run -d --name keep-me --label openshell.workspace-probe=true alpine sleep 600, then start an OpenShell gateway using the same Podman socket; startup reaches PodmanComputeDriver::new and issues DELETE for keep-me. A deterministic unit version is a Podman stub returning one ContainerListEntry with that label but no OpenShell ownership marker, and asserting that the constructor must not send DELETE.

PR ownership: This PR introduces the workspace-probe label, startup recovery path, and force-removal call; the base tree had no stale workspace probes to reconcile and no startup deletion selected by this label.

Requested change: Make probe cleanup ownership- and state-constrained: mark actual probe containers with standard OpenShell-managed ownership metadata plus a probe-specific identifier, and have startup recovery remove only stopped/exited OpenShell-owned probe containers that match those constraints. Add tests proving a non-managed container with openshell.workspace-probe=true is preserved and an active probe from another live driver instance is not removed.

@matthewgrossman matthewgrossman added the gator:in-review Gator is reviewing or awaiting PR review feedback label Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@maxamillion maxamillion Awaiting requested review from maxamillion maxamillion is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

gator:in-review Gator is reviewing or awaiting PR review feedback test:e2e Requires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@matthewgrossman